Skip to content

perf: Use Apify-provided environment variables to obtain PPE pricing information #483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Oct 9, 2025
Merged

perf: Use Apify-provided environment variables to obtain PPE pricing information #483

merged 6 commits into from
Oct 9, 2025
+128 −47

Conversation

janbuchar
Copy link
Contributor

@github-actions github-actions bot added this to the 125th sprint - Tooling team milestone Oct 7, 2025
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 7, 2025
@B4nan
Copy link
Member

B4nan commented Oct 7, 2025

Let's call this perf optimization, that's the main point of why we do it, right?

@B4nan B4nan changed the title refactor: Use Apify-provided environment variables to obtain PPE pricing information perf: Use Apify-provided environment variables to obtain PPE pricing information Oct 7, 2025
@janbuchar
Copy link
Contributor Author

E2E test is failing - I probably broke the charge limit logic

metalwarrior665
metalwarrior665 approved these changes Oct 7, 2025
View reviewed changes
Copy link
Member

@metalwarrior665 metalwarrior665 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the tested Temporary label used only programatically for some analytics. label Oct 8, 2025
@janbuchar
Copy link
Contributor Author

https://github.com/apify/apify-sdk-js/actions/runs/18348147670 e2e test run - should be OK now

@janbuchar janbuchar requested review from B4nan and barjin October 8, 2025 14:46
barjin
barjin approved these changes Oct 8, 2025
View reviewed changes
Copy link
Contributor

@barjin barjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you! 👍

I have a few ideas about the code quality, but neither is strictly necessary

@janbuchar janbuchar requested a review from barjin October 9, 2025 11:48
barjin
barjin approved these changes Oct 9, 2025
View reviewed changes
Copy link
Contributor

@barjin barjin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A smallish nit regarding the PPE testing mode, otherwise lgtm 👍

@janbuchar janbuchar merged commit 98dd09b into master Oct 9, 2025
9 checks passed
@janbuchar janbuchar deleted the use-ppe-env-vars branch October 9, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metalwarrior665 metalwarrior665 metalwarrior665 approved these changes

@barjin barjin barjin approved these changes

@B4nan B4nan Awaiting requested review from B4nan

Assignees

@janbuchar janbuchar

Labels
t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Milestone
125th sprint - Tooling team
Development

Successfully merging this pull request may close these issues.

Load Actor pricing and charged events from env vars
4 participants
@janbuchar @B4nan @metalwarrior665 @barjin